refactor: use useSWR in useUpdateUsdcBalances #2164
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| import { useBalances } from '../useBalances' | ||
| import { getProviderForChainId } from '@/token-bridge-sdk/utils' | ||
|
|
||
| export async function childChainUsdcAddressFetcher([ |
There was a problem hiding this comment.
Could we do something like this, also renaming the method? I see that it's easier to pass directly from SWR with an array, but it's just harder to read when calling the method.
getChildUsdcAddress({
parentUsdcAddress,
parentChainId,
childChainId
)}nit: I don't think we need to usd chain in variable names, it's implied. We've been using it without chain on the bridge already and everywhere in the sdk. Might wanna follow the same structure.
| }) | ||
| } | ||
|
|
||
| export function useParentChainUsdcAddress() { |
There was a problem hiding this comment.
IF you decide to go with variable names without 'chain', can rename this to useParentUsdcAddress. Otherwise keep consistent.
| export function useUpdateUsdcBalances({ | ||
| walletAddress | ||
| }: { | ||
| walletAddress: string | undefined |
There was a problem hiding this comment.
nit
| walletAddress: string | undefined | |
| walletAddress: Address | undefined |
| getL2ERC20Address: jest.fn() | ||
| })) | ||
|
|
||
| const xaiTestnetChainId = 37714555429 |
There was a problem hiding this comment.
nit: you could cast all of these as ChainId here and won't have to do it every time later
| ]) | ||
|
|
||
| expect(result).toEqual(CommonAddress.ArbitrumOne.USDC) | ||
| expect(result).not.toEqual(CommonAddress.ArbitrumOne['USDC.e']) |
There was a problem hiding this comment.
nit: we don't need to check for USDC.e, if USDC passed then USDC.e must fail (unless we change ArbitrumOne['USDC.e'] to the same value)
| import { getProviderForChainId } from '@/token-bridge-sdk/utils' | ||
|
|
||
| export async function childChainUsdcAddressFetcher([ | ||
| _parentChainUsdcAddress, |
There was a problem hiding this comment.
I wonder if we can do something about _parentChainUsdcAddress, it should not be required for non-Orbit chains, so we should be able to just call the method without specifying parent USDC address and rely on chain ID only.
Maybe we don't need useParentChainUsdcAddress, we could just make it a util method and use it here as well?
packages/arb-token-bridge-ui/src/hooks/CCTP/useUpdateUsdcBalances.ts
Outdated
Show resolved
Hide resolved
packages/arb-token-bridge-ui/src/hooks/CCTP/useUpdateUsdcBalances.ts
Outdated
Show resolved
Hide resolved
| updateUsdcBalances() | ||
| return | ||
| } | ||
|
|
||
| latestTokenBridge?.current?.token?.updateTokenData(selectedToken.address) |
There was a problem hiding this comment.
Just thinking here, can we maybe call updateUSDCBalances in updateTokenData directly?
There was a problem hiding this comment.
supposedly, i think it requires some exploration and so i haven't created any tickets for it or dealt with it on this PR as it's out of scope
packages/arb-token-bridge-ui/src/hooks/CCTP/useUpdateUsdcBalances.ts
Outdated
Show resolved
Hide resolved
packages/arb-token-bridge-ui/src/hooks/__tests__/useUpdateUsdcBalances.test.ts
Outdated
Show resolved
Hide resolved
packages/arb-token-bridge-ui/src/hooks/__tests__/useUpdateUsdcBalances.test.ts
Outdated
Show resolved
Hide resolved
packages/arb-token-bridge-ui/src/components/TransferPanel/TransferPanelMain.tsx
Outdated
Show resolved
Hide resolved
| }) | ||
| } | ||
|
|
||
| export function getParentUsdcAddress(parentChainId: number) { |
There was a problem hiding this comment.
we can use the CommonAddress object directly if we use chain ids as keys instead of the names Ethereum, Sepolia etc
however such refactoring will involve many files so i'm not doing it on this PR
We don't need to fetch the USDC address on the child chain every time we want to update the USDC balances because it will never change
Closes FS-1082